Initial POC for modify-image #3136

yeazelm · 2023-05-23T17:29:15Z

Issue number: 2486

Closes #

Description of changes:
This adds in a target for import-images and modify-image with basic support. There is more to come around SELinux and tidying up how we treat CA Bundles and tracking the changes.

This is incomplete and needs more testing, but is in an MVP of sorts. There are comments about what to do about incomplete files for bringing in the entire repo. Ideally, import-images moves into buildsys or pubsys anyway, so this is more of just a quick helper for the time being anyway.

There is more work to do around importing the kmod-kit and migrations for a complete treatment of importing artifacts one might need, but for now, this solves the "how do I modify an image" problem.

Testing done:
Validated with metal images via calling:

cargo make -e BUILDSYS_VERSION_BUILD=1.12.0-6ef1139f -e BUILDSYS_VARIANT=metal-k8s-1.25 -e BUILDSYS_ARCH=x86_64 -e BUILDSYS_CA_BUNDLE=roles/default.root.json import-images

cargo make -e BUILDSYS_VERSION_BUILD=1.12.0-6ef1139f -e BUILDSYS_VARIANT=metal-k8s-1.25 -e BUILDSYS_ARCH=x86_64 -e BUILDSYS_CA_BUNDLE=roles/default.root.json modify-image

Debugfs shows files have the right context:

$ debugfs bottlerocket-metal-k8s-1.25-x86_64-1.12.0-6ef1139f-root.ext4
debugfs 1.46.5 (30-Dec-2021)
debugfs:  ea_list /usr/share/bottlerocket/trust-roots.json
Extended attributes:
  security.selinux (25) = "system_u:object_r:os_t:s0"
debugfs:  ea_list /usr/share/factory/etc/pki/tls/certs/ca-bundle.crt
Extended attributes:
  security.selinux (25) = "system_u:object_r:os_t:s0"
debugfs:  ea_list /usr/share/factory/etc/pki/tls/certs/ca-bundle.crt^C
debugfs:  ea_list /usr/share/updog/root.json
Extended attributes:
  security.selinux (25) = "system_u:object_r:os_t:s0"

The /usr/share/bottlerocket/trust-roots.json has the contents updated:

{
  "ModifiedRootsOfTrust": [
    {
      "Path": "/usr/share/updog/root.json",
      "NewHash": "b81af4d8eb86743539fbc4709d33ada7b118d9f929f0c2f6c04e1d41f46241ed80423666d169079d736ab79965b4dd25a5a6db5f01578b397496d49ce11a3aa2",
      "OldHash": "b81af4d8eb86743539fbc4709d33ada7b118d9f929f0c2f6c04e1d41f46241ed80423666d169079d736ab79965b4dd25a5a6db5f01578b397496d49ce11a3aa2"
    },
    {
      "Path": "/usr/share/factory/etc/pki/tls/certs/ca-bundle.crt",
      "NewHash": "bbd3521d79c3897aa7cd937b486369665b5c2a654401cc2151a63453b805fbb2d2484478eb195e358d2b100ee7422b6868c5d285c8ca22191cbd6a242a9ccfa6",
      "OldHash": "fbbd8d33932a5d65dd548d91927fc5bac5218d5a44b8d992591bef2eab22b09cc2154b6effb2df1c61e1aa233816e3c3e7acfb27b3e3f90672a7752bb05b710f"
    }
  ]
}

Terms of contribution:

By submitting this pull request, I agree that this contribution is dual-licensed under the terms of both the Apache License, version 2.0, and the MIT license.

This adds in a target for import-images and modify-image with basic support. There is more to come around SELinux and tidying up how we treat CA Bundles and tracking the changes.

webern · 2023-05-24T17:30:06Z

Makefile.toml

+#bottlerocket-${BUILDSYS_VARIANT}-${BUILDSYS_ARCH}-${VERSION}-${HASH}-root.verity.lz4
+
+if [ -s "${bootlz4}" ] || [ -s "${imglz4}" ] || [ -s "${rootlz4}" ] || [ -s "${hashlz4}" ]; then
+   echo "Image files already exist for the current version/commit - ${IMAGE} - please run 'cargo make clean' to remove them" >&2


Maybe we should have a clean-images task so we don't blow out all the docker and rpm stuff.

Yeah, that seems reasonable. I was focusing on CI/CD flows but it stands to reason one might have other builds in their directory that they don't want cleaned.

webern · 2023-05-24T17:31:41Z

Makefile.toml

+# variant-arch-version
+# variant-arch-version(with-v)
+VERSION=$(echo "${BUILDSYS_VERSION_BUILD}"| cut -f1 -d"-")
+ln -s "${OUTDIR}/${bootlz4}" "${OUTDIR}/bottlerocket-${BUILDSYS_VARIANT}-${BUILDSYS_ARCH}-boot.ext4.lz"


This seems problematic in that you need to have the right sha checked out for this to match what was downloaded, right?

Are we letting the symlinks point at this from our current git sha even if it doesn't match the git sha of the downloaded image?

This seems problematic in that you need to have the right sha checked out for this to match what was downloaded, right?

No, you don't actually need the SHA checked out which is one of the primary desires of this logic, to avoid having to fiddle with getting that just right, instead you simply need to know the version/hash to download from the repo and then it will create it, this makes me realize I didn't show the structure of the build directory:

$ ls -al build/images/x86_64-metal-k8s-1.25 ... ... 1.12.0-6ef1139f ... latest -> 1.12.0-6ef1139f $ ls -al build/images/x86_64-metal-k8s-1.25/1.12.0-6ef1139f ... bottlerocket-metal-k8s-1.25-x86_64-1.12.0-6ef1139f-boot.ext4 bottlerocket-metal-k8s-1.25-x86_64-1.12.0-6ef1139f-boot.ext4.lz4 bottlerocket-metal-k8s-1.25-x86_64-1.12.0-6ef1139f.img.lz4 bottlerocket-metal-k8s-1.25-x86_64-1.12.0-6ef1139f-root.ext4 bottlerocket-metal-k8s-1.25-x86_64-1.12.0-6ef1139f-root.ext4.lz4 bottlerocket-metal-k8s-1.25-x86_64-1.12.0-6ef1139f-root.verity.lz4 bottlerocket-metal-k8s-1.25-x86_64-1.12.0-boot.ext4.lz -> /storage/yeazelm/git/yeazelm-br/build/images/x86_64-metal-k8s-1.25/1.12.0-6ef1139f/bottlerocket-metal-k8s-1.25-x86_64-1.12.0-6ef1139f-boot.ext4.lz4 bottlerocket-metal-k8s-1.25-x86_64-1.12.0.img.lz4 -> /storage/yeazelm/git/yeazelm-br/build/images/x86_64-metal-k8s-1.25/1.12.0-6ef1139f/bottlerocket-metal-k8s-1.25-x86_64-1.12.0-6ef1139f.img.lz4 bottlerocket-metal-k8s-1.25-x86_64-1.12.0-root.ext4.lz4 -> /storage/yeazelm/git/yeazelm-br/build/images/x86_64-metal-k8s-1.25/1.12.0-6ef1139f/bottlerocket-metal-k8s-1.25-x86_64-1.12.0-6ef1139f-root.ext4.lz4 bottlerocket-metal-k8s-1.25-x86_64-1.12.0-root.verity.lz4 -> /storage/yeazelm/git/yeazelm-br/build/images/x86_64-metal-k8s-1.25/1.12.0-6ef1139f/bottlerocket-metal-k8s-1.25-x86_64-1.12.0-6ef1139f-root.verity.lz4 bottlerocket-metal-k8s-1.25-x86_64-boot.ext4.lz -> /storage/yeazelm/git/yeazelm-br/build/images/x86_64-metal-k8s-1.25/1.12.0-6ef1139f/bottlerocket-metal-k8s-1.25-x86_64-1.12.0-6ef1139f-boot.ext4.lz4 bottlerocket-metal-k8s-1.25-x86_64.img.lz4 -> /storage/yeazelm/git/yeazelm-br/build/images/x86_64-metal-k8s-1.25/1.12.0-6ef1139f/bottlerocket-metal-k8s-1.25-x86_64-1.12.0-6ef1139f.img.lz4 bottlerocket-metal-k8s-1.25-x86_64-root.ext4.lz4 -> /storage/yeazelm/git/yeazelm-br/build/images/x86_64-metal-k8s-1.25/1.12.0-6ef1139f/bottlerocket-metal-k8s-1.25-x86_64-1.12.0-6ef1139f-root.ext4.lz4 bottlerocket-metal-k8s-1.25-x86_64-root.verity.lz4 -> /storage/yeazelm/git/yeazelm-br/build/images/x86_64-metal-k8s-1.25/1.12.0-6ef1139f/bottlerocket-metal-k8s-1.25-x86_64-1.12.0-6ef1139f-root.verity.lz4 bottlerocket-metal-k8s-1.25-x86_64-v1.12.0-boot.ext4.lz -> /storage/yeazelm/git/yeazelm-br/build/images/x86_64-metal-k8s-1.25/1.12.0-6ef1139f/bottlerocket-metal-k8s-1.25-x86_64-1.12.0-6ef1139f-boot.ext4.lz4 bottlerocket-metal-k8s-1.25-x86_64-v1.12.0.img.lz4 -> /storage/yeazelm/git/yeazelm-br/build/images/x86_64-metal-k8s-1.25/1.12.0-6ef1139f/bottlerocket-metal-k8s-1.25-x86_64-1.12.0-6ef1139f.img.lz4 bottlerocket-metal-k8s-1.25-x86_64-v1.12.0-root.ext4.lz4 -> /storage/yeazelm/git/yeazelm-br/build/images/x86_64-metal-k8s-1.25/1.12.0-6ef1139f/bottlerocket-metal-k8s-1.25-x86_64-1.12.0-6ef1139f-root.ext4.lz4 bottlerocket-metal-k8s-1.25-x86_64-v1.12.0-root.verity.lz4 -> /storage/yeazelm/git/yeazelm-br/build/images/x86_64-metal-k8s-1.25/1.12.0-6ef1139f/bottlerocket-metal-k8s-1.25-x86_64-1.12.0-6ef1139f-root.verity.lz4

Are we letting the symlinks point at this from our current git sha even if it doesn't match the git sha of the downloaded image?

We move the symlinks to that point in time specifically for that version within the build, this is always the case for a particular build with a SHA, but won't affect a different builds. This is all within the versioned directory. We could avoid touching the build/images/${VARIANT_NAME}/latest link but I found it useful for finding the right thing. Its debatable if its "right" or not.

webern · 2023-05-24T17:38:37Z

Makefile.toml

+'''
+]
+
+[tasks.modify-image-shell]


I don't get what this task is in relation to modify-image

This has been here to simulate the env of modify-image but be interactive. It won't make the final PR but is here for debugging while I work through the issues.

bcressey · 2023-07-25T15:20:05Z

Makefile.toml

+# Build path
+tuftool download \
+    "${OUTDIR}" \
+    --target-name "${bootlz4}" \
+    --target-name "${imglz4}" \
+    --target-name "${rootlz4}" \
+    --target-name "${hashlz4}" \
+    --root ./root.json \
+    --metadata-url "https://updates.bottlerocket.aws/2020-07-07/${BUILDSYS_VARIANT}/${BUILDSYS_ARCH}/" \
+    --targets-url "https://updates.bottlerocket.aws/targets/"


I'd recommend splitting this up into two parts, maybe two tasks:

a pubsys invocation to pull down the images into the build/repos structure

a buildsys invocation to import images from that local structure into the build/images structure

The current convention is for tuftool invocations to be wrapped by pubsys and I'd like to keep that.

I feel stronger about the buildsys approach since ideally we'd use similar Dockerfile targets and refactor out the shell functions to handle the symlinks.

This makes sense. I mostly wrote this to help me test the actual modification flow but I agree putting this into pubsys and buildsys is a better place. I'll add some tasks to get that work done.

bcressey · 2023-07-25T15:21:49Z

Makefile.toml

+docker run --rm -it \
+  --user "$(id -u):$(id -g)" \
+  --security-opt label:disable \
+  -e CARGO_HOME="/tmp/.cargo" \
+  -v "${CARGO_HOME}":/tmp/.cargo \
+  -v "${BUILDSYS_ROOT_DIR}/tools":/tmp/tools \
+  -v "${BUILDSYS_OUTPUT_DIR}":/tmp/build \
+  -v "${CA_BUNDLE}":/tmp/ca-bundle.crt \
+  -v "${NEW_ROOT_JSON}":/tmp/root.json \
+  "${BUILDSYS_SDK_IMAGE}" \
+  bash -c "${run_modify_image}"


I would like this to be another buildsys invocation.

I had not looked at buildsys very deeply when writing this but after looking at buildsys more I think I understand where this would live. Are you thinking of this as a third stage in the Dockerfile as well or some other path I'm not seeing for this logic?

bcressey · 2023-07-25T15:27:36Z

tools/modify-image

+ROOT_START="$(sgdisk --print "${IMAGE}" | awk '/BOTTLEROCKET-ROOT-A/{ print $2 }')"
+echo ${ROOT_START}
+ROOT_END="$(sgdisk --print "${IMAGE}" | awk '/BOTTLEROCKET-ROOT-A/{ print $3 }')"
+echo ${ROOT_END}
+ROOT_SIZE="$(( ROOT_END - ROOT_START + 1))"


It might help to associate units with these - sectors, bytes, whatever - since it's not immediately obvious what the fields we're cutting out of sgdisk output. If sgdisk --info lets us match lines by a prefix then that might help too.

Maybe a helper function that takes a partition name and makes a couple sgdisk calls to find it and populate the variables that the caller passes in.

Good call out, I like the helper function idea!

bcressey · 2023-07-25T15:29:02Z

tools/modify-image

+# Create the full manifest of modifications for writing to trust-roots.json
+echo -e "The full list of json is: ${CHANGED_ROOTS[@]}"
+MODIFIED_JSON="$(jq --raw-output . <<<  "${CHANGED_ROOTS[@]}")"
+
+FULL_MODIFIED_JSON="$(jq --slurp 'sort_by(.Name)' <<< "${MODIFIED_JSON}" | jq '{"ModifiedRootsOfTrust": .}')"


I have doubts about creating this manifest as a file in the image, but I could see adding this as an additional output for any newly built or modified image.

I think the primary value of this file is for image self-reference. One can use the image or booted OS directly to determine what is present and not rely upon build logs to determine this. I think of this similarly to the application-inventory.json where this data can be gleaned from the git repo and build logs but it can be convenient to have this data directly in the image. The main difference between the application inventory and this file is that this trust-roots file could be easily generated from the image/files on the filesystem directly. So if one does care about these files, they can find them and hash them themselves. So all in all, this is mostly a nice to have and the build output is probably valuable and I could be convinced to not include it in the image directly.

yeazelm · 2023-07-24T18:14:55Z

tools/modify-image

+      --image=*) IMAGE_PATH="${optarg}" ;;
+      --image-out=*) OUT_IMAGE="${optarg}" ;;
+      --new-root=*) ROOTFILE="${optarg}" ;;
+      --ca-bundle=*) CABUNDLE="${optarg}" ;;


This doesn't solve for a few use cases we probably need, notably:

Secure Boot

OVA image formats

yeazelm · 2023-07-25T15:45:40Z

Makefile.toml

+# Build path
+tuftool download \
+    "${OUTDIR}" \
+    --target-name "${bootlz4}" \
+    --target-name "${imglz4}" \
+    --target-name "${rootlz4}" \
+    --target-name "${hashlz4}" \
+    --root ./root.json \
+    --metadata-url "https://updates.bottlerocket.aws/2020-07-07/${BUILDSYS_VARIANT}/${BUILDSYS_ARCH}/" \
+    --targets-url "https://updates.bottlerocket.aws/targets/"


This makes sense. I mostly wrote this to help me test the actual modification flow but I agree putting this into pubsys and buildsys is a better place. I'll add some tasks to get that work done.

yeazelm · 2023-07-25T15:47:17Z

Makefile.toml

+docker run --rm -it \
+  --user "$(id -u):$(id -g)" \
+  --security-opt label:disable \
+  -e CARGO_HOME="/tmp/.cargo" \
+  -v "${CARGO_HOME}":/tmp/.cargo \
+  -v "${BUILDSYS_ROOT_DIR}/tools":/tmp/tools \
+  -v "${BUILDSYS_OUTPUT_DIR}":/tmp/build \
+  -v "${CA_BUNDLE}":/tmp/ca-bundle.crt \
+  -v "${NEW_ROOT_JSON}":/tmp/root.json \
+  "${BUILDSYS_SDK_IMAGE}" \
+  bash -c "${run_modify_image}"


I had not looked at buildsys very deeply when writing this but after looking at buildsys more I think I understand where this would live. Are you thinking of this as a third stage in the Dockerfile as well or some other path I'm not seeing for this logic?

yeazelm · 2023-07-25T15:48:22Z

tools/modify-image

+ROOT_START="$(sgdisk --print "${IMAGE}" | awk '/BOTTLEROCKET-ROOT-A/{ print $2 }')"
+echo ${ROOT_START}
+ROOT_END="$(sgdisk --print "${IMAGE}" | awk '/BOTTLEROCKET-ROOT-A/{ print $3 }')"
+echo ${ROOT_END}
+ROOT_SIZE="$(( ROOT_END - ROOT_START + 1))"


Good call out, I like the helper function idea!

yeazelm · 2023-07-25T15:59:03Z

tools/modify-image

+# Create the full manifest of modifications for writing to trust-roots.json
+echo -e "The full list of json is: ${CHANGED_ROOTS[@]}"
+MODIFIED_JSON="$(jq --raw-output . <<<  "${CHANGED_ROOTS[@]}")"
+
+FULL_MODIFIED_JSON="$(jq --slurp 'sort_by(.Name)' <<< "${MODIFIED_JSON}" | jq '{"ModifiedRootsOfTrust": .}')"


I think the primary value of this file is for image self-reference. One can use the image or booted OS directly to determine what is present and not rely upon build logs to determine this. I think of this similarly to the application-inventory.json where this data can be gleaned from the git repo and build logs but it can be convenient to have this data directly in the image. The main difference between the application inventory and this file is that this trust-roots file could be easily generated from the image/files on the filesystem directly. So if one does care about these files, they can find them and hash them themselves. So all in all, this is mostly a nice to have and the build output is probably valuable and I could be convinced to not include it in the image directly.

webern

Blocking for Makefile.toml changes.

Initial POC for modify-image

41a1811

This adds in a target for import-images and modify-image with basic support. There is more to come around SELinux and tidying up how we treat CA Bundles and tracking the changes.

yeazelm force-pushed the dm-verity branch from b0cb4d1 to 41a1811 Compare May 23, 2023 21:41

webern reviewed May 24, 2023

View reviewed changes

bcressey reviewed Jul 25, 2023

View reviewed changes

yeazelm commented Jul 25, 2023

View reviewed changes

webern suggested changes Aug 18, 2023

View reviewed changes

yeazelm closed this Apr 10, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Initial POC for modify-image #3136

Initial POC for modify-image #3136

yeazelm commented May 23, 2023

webern May 24, 2023

yeazelm May 24, 2023

webern May 24, 2023

yeazelm May 24, 2023

webern May 24, 2023

yeazelm May 24, 2023

bcressey Jul 25, 2023

yeazelm Jul 25, 2023

bcressey Jul 25, 2023

yeazelm Jul 25, 2023

bcressey Jul 25, 2023

yeazelm Jul 25, 2023

bcressey Jul 25, 2023

yeazelm Jul 25, 2023

yeazelm Jul 24, 2023

yeazelm Jul 25, 2023

yeazelm Jul 25, 2023

yeazelm Jul 25, 2023

yeazelm Jul 25, 2023

webern left a comment

Initial POC for modify-image #3136

Initial POC for modify-image #3136

Conversation

yeazelm commented May 23, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

webern left a comment

Choose a reason for hiding this comment